-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade packages #107
Upgrade packages #107
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have to merge this PR when we do the same upgrades for the packages with some other internal packages.
setup.cfg
Outdated
@@ -1,6 +1,6 @@ | |||
[metadata] | |||
name = df_to_azure | |||
version = 0.6.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have to bump this to version 0.7.0. Since potentially there can be changes in functionality with upgrading the packages.
setup.cfg
Outdated
azure-mgmt-datafactory==2.2.0 | ||
azure-mgmt-resource==20.1.0 | ||
azure-storage-blob==12.9.0 | ||
pandas==1.4.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should align this with some other internal packages, since we will get difference in pandas versions between them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pandas version is equal to azure-batchload, do you seem any other problems? Because now the release candidate is not usable in combination with azure-batchload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what sense is not usable? I think the point is that if we upgrade the packages here, we should upgrade them in the other packages as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a mismatch in pandas dependencies, it might work, but it could be unexpected, not what you want.
Which ones should be in scope for the 'other' packages (azure-batch-load matches with these changes), and why should we do that first (and not after we update df-to-azure in those packages)?
My proposal would be to make the requirements use ~=. @erfannariman what do you think? |
Yes let's go for it and see if that solves the problem. |
Done |
In this PR
I've run the tests locally, without errors. I've had some issues with packages like pandas and pyodbc on the new MacBook M1. The newest versions of pandas and pyodbc seem to be working. I took the opportunity to upgrade all other packages as well.